Port clocks and filesystems functions to use native wasip2 operations #606

catamorphism · 2025-08-05T20:58:26Z

At this point, there should be no further uses of the preview1 component adapter for clocks and filesystems methods when __wasilibc_use_wasip2 is defined.

Also added a DEBUG option to the Makefile that enables -O0/g when DEBUG=true is passed in.

catamorphism · 2025-08-05T21:00:10Z

Note: there's no automated way (that I know of) to check that there are no uses of the preview1 filesystems methods when __wasilibc_use_wasip2 is defined, so I checked by searching for each of the method names and verifying that it was enclosed in an appropriate #ifdef. It's possible I could have missed something.

pchickey

So far reviewed clocks, this one will take time to go through piece by piece

libc-bottom-half/clocks/clock.c

libc-bottom-half/cloudlibc/src/common/time.h

At this point, there should be no further uses of the preview1 component adapter for clocks and filesystems methods when __wasilibc_use_wasip2 is defined. Also added a DEBUG option to the Makefile that enables -O0/g when DEBUG=true is passed in.

Co-authored-by: Pat Hickey <[email protected]>

pchickey

Today I made it up to the end of the libc itself, and I skimmed the tests. I'll give the tests another read-through later.

It appears __wasilibc_fd_renumber isnt ported in this PR - that should be possible to implement just by manipulating libc's own descriptor table

pchickey · 2025-08-22T23:37:33Z

libc-bottom-half/cloudlibc/src/libc/dirent/readdir.c

+  }
+
+  if (!dir_entry_optional.is_some) {
+    // End-of-file


Need to close the stream and remove it from the descriptor table - otherwise its effectively leaked. (And in the error case as well)

pchickey · 2025-08-22T23:45:56Z

libc-bottom-half/cloudlibc/src/libc/fcntl/openat.c

+        fs_flags |= FILESYSTEM_DESCRIPTOR_FLAGS_READ;
+      }
+      if ((oflag & O_WRONLY) != 0) {
+          // Sync flags are not supported yet


Is this a TODO, or an assertion about implementations? I think you'd want to take care of the presence or absence of O_SYNC and translating that to a descriptor flag below this switch

pchickey · 2025-08-22T23:54:29Z

libc-bottom-half/cloudlibc/src/libc/sys/ioctl/ioctl.c

@@ -110,6 +111,11 @@ int ioctl(int fildes, int request, ...) {
      return 0;
    }
    case FIONBIO: {
+#ifdef __wasilibc_use_wasip2
+      // wasip2 doesn't support setting the non-blocking flag


Thats correct, and for files this doesn't matter, but if this was a socket, I think we'd want to emulate setting the nonblocking flag by switching the implementations of send/recv from blocking to nonblocking stream read/writes. But just googling around it seems like this particular ioctl has a checkered past so maybe it doesn't matter if we just say ENOTSUP? Genuinely not sure what the right thing is to do here.

pchickey · 2025-08-22T23:56:51Z

libc-bottom-half/cloudlibc/src/libc/sys/select/pselect.c

@@ -61,14 +61,24 @@ int pselect(int nfds, fd_set *restrict readfds, fd_set *restrict writefds,

  int poll_timeout;
  if (timeout) {
+#ifdef __wasilibc_use_wasip2
+    wall_clock_datetime_t timestamp;


I think we want to use the monotonic clock for timeouts instead of the wall clock, since its about measuring duration

pchickey · 2025-08-23T00:13:28Z

libc-bottom-half/cloudlibc/src/libc/sys/uio/writev.c

+
+#ifdef __wasilibc_use_wasip2
+  // Following the behavior of the preview1 component adapter,
+  // only write the first non-empty iov


I don't have super strong feelings about this, but I'm fine with trying to write as much as possible here by iterating through the iovs instead of following the component adapter's behavior, which has surprised some users.

pchickey · 2025-08-23T00:17:25Z

libc-bottom-half/cloudlibc/src/libc/time/clock_nanosleep.c

+  if ((flags & ~TIMER_ABSTIME) != 0)
+    return EINVAL;
+
+  // Prepare polling subscription.


I guess this is fine for this PR but I do see its rewritten properly in #608 so all good.

pchickey · 2025-08-23T00:19:13Z

libc-bottom-half/cloudlibc/src/libc/unistd/lseek.c

+      return -1;
+    }
+  }
+  return bytes_skipped;


should this be total_bytes_skipped?

pchickey · 2025-08-23T00:23:00Z

libc-bottom-half/cloudlibc/src/libc/unistd/lseek.c

+      offset_to_use = offset + entry->stream.offset;
+      break;
+    case SEEK_END: {
+      // Find the end of the stream (is there a better way to do this?)


For file streams, I think the efficient way to implement this is to get the filestat and use the len field.

pchickey · 2025-08-28T16:14:52Z

libc-bottom-half/cloudlibc/src/libc/unistd/write.c

+
+      // Get the output stream
+      filesystem_own_output_stream_t stream_owned;
+      ok = filesystem_method_descriptor_write_via_stream(entry->file.file_handle,


Can we cache a stream per file_handle to be used by write, rather than open a new one each write?

pchickey · 2025-08-28T16:14:58Z

libc-bottom-half/cloudlibc/src/libc/unistd/write.c

+
+  if (num_bytes_permitted < nbyte) {
+    streams_own_pollable_t pollable = streams_method_output_stream_subscribe(output_stream);
+    poll_method_pollable_block(poll_borrow_pollable(pollable));


This pollable gets leaked - it needs a drop call. But also, can we cache it for next time this output stream is used? Creating a new pollable and dropping it each time creates more churn than ideal.

pchickey · 2025-08-28T16:15:39Z

libc-bottom-half/cloudlibc/src/libc/unistd/write.c

+  if (fildes == 1)
+    output_stream = streams_borrow_output_stream(stdout_get_stdout());
+  else if (fildes == 2)
+    output_stream = streams_borrow_output_stream(stderr_get_stderr());


Can we cache these streams, as well, so that libc only needs to call get_stdout and get_stderr once for the lifetime of the instance?

pchickey · 2025-08-28T16:15:57Z

libc-bottom-half/headers/private/wasi/descriptor_table.h

 // This is a tagged union. When adding/removing/renaming cases, be sure to keep the tag and union definitions in sync.
 typedef struct {
 	enum {
 		DESCRIPTOR_TABLE_ENTRY_TCP_SOCKET,
 		DESCRIPTOR_TABLE_ENTRY_UDP_SOCKET,
+                DESCRIPTOR_TABLE_ENTRY_FILE_HANDLE,
+                DESCRIPTOR_TABLE_ENTRY_DIRECTORY_STREAM,
+                DESCRIPTOR_TABLE_ENTRY_FILE_STREAM


Mix of tabs/spaces

catamorphism force-pushed the wasip2-porting branch from 6f9de1e to 76e18ad Compare August 5, 2025 21:23

catamorphism marked this pull request as ready for review August 5, 2025 21:35

catamorphism force-pushed the wasip2-porting branch from 76e18ad to 39e65a4 Compare August 6, 2025 23:58

catamorphism mentioned this pull request Aug 7, 2025

DRAFT: Port polling operations to wasip2 #608

Draft

catamorphism force-pushed the wasip2-porting branch from e51b64f to bf042e2 Compare August 9, 2025 00:15

pchickey self-requested a review August 11, 2025 17:32

pchickey requested changes Aug 13, 2025

View reviewed changes

libc-bottom-half/clocks/clock.c Outdated Show resolved Hide resolved

libc-bottom-half/cloudlibc/src/common/time.h Outdated Show resolved Hide resolved

libc-bottom-half/cloudlibc/src/common/time.h Outdated Show resolved Hide resolved

catamorphism and others added 5 commits August 15, 2025 13:08

Port time() and times()

261897d

Update libc-bottom-half/cloudlibc/src/common/time.h

60f063a

Co-authored-by: Pat Hickey <[email protected]>

Update libc-bottom-half/cloudlibc/src/common/time.h

2051b38

Co-authored-by: Pat Hickey <[email protected]>

Use monotonic clock where appropriate

b3c729d

catamorphism force-pushed the wasip2-porting branch from a1a9296 to b3c729d Compare August 15, 2025 20:08

pchickey reviewed Aug 23, 2025

View reviewed changes

pchickey reviewed Aug 28, 2025

View reviewed changes

Port clocks and filesystems functions to use native wasip2 operations #606

Are you sure you want to change the base?

Port clocks and filesystems functions to use native wasip2 operations #606

Conversation

catamorphism commented Aug 5, 2025

Uh oh!

catamorphism commented Aug 5, 2025

Uh oh!

pchickey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pchickey left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!